-
Notifications
You must be signed in to change notification settings - Fork 24
[#603] Refactor to use Material3 #605
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[#603] Refactor to use Material3 #605
Conversation
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Kover report for template-compose:🧛 Template - Compose Unit Tests Code Coverage:
|
File | Coverage |
---|---|
AppColors.kt |
100.00% |
AppShapes.kt |
100.00% |
AppTypography.kt |
100.00% |
HomeScreen.kt |
67.89% |
Theme.kt |
46.43% |
Modified Files Not Found In Coverage Report:
AppBar.kt
Item.kt
ItemList.kt
SecondScreen.kt
ThirdScreen.kt
libs.versions.toml
libs.versions.toml
Codebase cunningly covered by count Shroud 🧛
Generated by 🚫 Danger
7254956
to
d91fffa
Compare
e4bc1db
to
ce96d87
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR upgrades both Template and Sample Compose projects to use Material3 by updating dependencies, refactoring theme definitions, and replacing Material imports/usages with their Material3 equivalents.
- Swap out
androidx.compose.material
forandroidx.compose.material3
dependencies and imports - Introduce
AppColors
,AppShapes
,AppTypography
interfaces backed by Material3 types - Update UI components (e.g.,
Text
,Scaffold
,Divider
,AppBar
) to use Material3 variants
Reviewed Changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
template-compose/gradle/libs.versions.toml | Replaced compose-material with compose-material3 dependency |
template-compose/app/src/main/java/.../ui/theme/Theme.kt | Refactored theme entrypoint to use Material3 MaterialTheme and new App* locals |
template-compose/app/src/.../ui/theme/AppColors.kt | Switched AppColors to wrap ColorScheme , introduced darkColorScheme /lightColorScheme |
sample-compose/gradle/libs.versions.toml | Replaced compose-material with compose-material3 dependency |
sample-compose/app/src/main/java/.../AppBar.kt | Updated TopAppBar to Material3 and adjusted coloring logic |
Comments suppressed due to low confidence (1)
sample-compose/app/src/main/java/co/nimblehq/sample/compose/ui/common/AppBar.kt:25
- TopAppBarColors is not a data class and does not have a
copy
method. UseTopAppBarDefaults.topAppBarColors(containerColor = AppTheme.colors.topAppBarBackground)
instead to set the container color.
colors = TopAppBarDefaults.topAppBarColors().copy(
DarkColorPalette | ||
} else { | ||
LightColorPalette | ||
} | ||
val typography = LocalAppTypography.current | ||
val shapes = LocalAppShapes.current | ||
colors.run { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mutating the shared AppColors
instance via colors.run { this.colorScheme = ... }
can introduce unintended side effects across compositions. Consider creating a fresh immutable AppColors
object per theme or passing a new instance into CompositionLocalProvider
instead of mutating the existing one.
Copilot uses AI. Check for mistakes.
text = { Text(stringResource(id = R.string.third_edit_menu_title)) }, | ||
onClick = { onLongClick(uiModel) } | ||
) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Redundant line
#603
What happened 👀
Insight 📝
N/A
Proof Of Work 📹
Both Template Compose and Sample Compose run the same
Screen_recording_20250228_143025.mp4
Screen_recording_20250228_143247.mp4